-
-
Notifications
You must be signed in to change notification settings - Fork 49
Fix name collisions in ArduinoISP sketch #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…bitbanged SPI mode for board using ArduinoCore-API When using bit banged SPI, which the sketch did when compiled for any architecture other than AVR, a `SPISettings` class was declared by the sketch. At the time the sketch was written, it was reasonable to expect this would not cause a name collision, since SPI.h is not `#include`d when doing bit banged SPI. However, since then a `SPISettings` class has been declared in [ArduinoCore-API's HardwareSPI.h](https://github.com/arduino/ArduinoCore-API/blob/932c7c7d4d4d334b10484284cc846672ad59607c/api/HardwareSPI.h#L37), causing the ArduinoISP sketch to not compile for any board whose core uses ArduinoCoreAPI (currently Arduino Mega AVR Boards, "Arduino nRF528x Boards (Mbed OS]", and "Arduino Mbed OS Boards (nRF52840 / STM32H747)"): ``` /github/workspace/examples/11.ArduinoISP/ArduinoISP/ArduinoISP.ino:191:27: error: reference to 'SPISettings' is ambiguous void beginTransaction(SPISettings settings) { ^~~~~~~~~~~ /github/workspace/examples/11.ArduinoISP/ArduinoISP/ArduinoISP.ino:167:7: note: candidates are: class SPISettings class SPISettings { ^~~~~~~~~~~ In file included from /github/home/.arduino15/packages/arduino/hardware/megaavr/1.8.6/cores/arduino/api/ArduinoAPI.h:31:0, from /github/home/.arduino15/packages/arduino/hardware/megaavr/1.8.6/cores/arduino/Arduino.h:23, from /github/workspace/examples/11.ArduinoISP/ArduinoISP/ArduinoISP.ino:39: /github/home/.arduino15/packages/arduino/hardware/megaavr/1.8.6/cores/arduino/api/HardwareSPI.h:37:7: note: class arduino::SPISettings class SPISettings { ^~~~~~~~~~~ ``` The fix is to use the `ARDUINO_API_VERSION` macro defined by ArduinoCore-API to detect when it is in use and make the bitbanged SPI code use the `SPISettings` class from ArduinoCore-API in this case.
…d OS The name collision between the variable named "error" in the ArduinoISP sketch and a function of that name declared by Mbed OS causes compilation of the sketch to fail for boards that use the "Arduino nRF528x Boards (Mbed OS)" or "Arduino Mbed OS Boards (nRF52840 / STM32H747)" cores.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, @per1234
One question: I saw you removed the friend class
directive, why was it there?
Previously, // Core developer MUST use an helper function in beginTransaction() to use this data So I just mirrored the way the I'm definitely open to feedback on this. I'm not an expert in this subject, and only took on the project because the alternative to fixing the compilation failure was excluding the ArduinoISP sketch from the CI, which very much didn't sit well with me. |
right! I never do that, always loved getter/setter strategies as I said, this looks well written |
Name collisions caused compilation of the ArduinoISP sketch to fail when: